Skip to content

refactor: route verify flagged through command module#160

Closed
ndycode wants to merge 3 commits intorefactor/pr1-verify-flagged-commandfrom
refactor/pr1-route-verify-flagged-direct
Closed

refactor: route verify flagged through command module#160
ndycode wants to merge 3 commits intorefactor/pr1-verify-flagged-commandfrom
refactor/pr1-route-verify-flagged-direct

Conversation

@ndycode
Copy link
Owner

@ndycode ndycode commented Mar 20, 2026

Summary

  • remove the remaining inline verify-flagged wrapper from lib/codex-manager.ts
  • route both CLI dispatch and login-menu problem-account checks through the extracted verify-flagged command module

What Changed

  • deleted the inline runVerifyFlagged() wrapper
  • updated codex auth verify-flagged dispatch to call runVerifyFlaggedCommand(...) directly
  • updated the login-menu verify-flagged action to use the same extracted command path

Validation

  • npm run test -- test/codex-manager-verify-flagged-command.test.ts test/codex-manager-cli.test.ts
  • npm run lint
  • npm run typecheck
  • npm run build

Risk and Rollback

  • Risk level: low
  • Rollback plan: revert c88cd18 to restore the inline verify-flagged wrapper

Additional Notes

  • this removes another dispatcher-only wrapper and keeps the stacked manager split moving in small reviewable slices

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

removes the last inline runVerifyFlagged dispatcher wrapper from codex-manager.ts and routes both the cli verify-flagged command and the login-menu problem-account check through runVerifyFlaggedCommand directly, using a new buildVerifyFlaggedCommandDeps() factory to construct the typed deps object.

changes:

  • runVerifyFlagged(args) wrapper deleted — it was a thin shell with no logic of its own
  • buildVerifyFlaggedCommandDeps(): VerifyFlaggedCommandDeps added as a typed factory; both call sites now pass buildVerifyFlaggedCommandDeps() inline
  • VerifyFlaggedCommandDeps type is now imported so the factory return type is statically checked against the command module's interface
  • new test "runs verify-flagged from the login menu" covers the login-menu dispatch path end-to-end: flags one account, refresh fails (invalid_grant), expects the transaction and saveFlaggedAccounts to be called exactly once

no behavioral change — the deps object passed to runVerifyFlaggedCommand is identical to what the old wrapper provided; default args (restore: true, dryRun: false) are unchanged and match the existing codex-manager-verify-flagged-command.test.ts coverage.

Confidence Score: 5/5

  • safe to merge — pure dispatcher refactor with no behavioral change and a new integration test covering the previously untested login-menu path
  • the deleted wrapper was a pass-through with identical deps; the factory function is statically typed against VerifyFlaggedCommandDeps; both call sites are mechanically equivalent to what existed before; the new test confirms the login-menu route executes the transaction and save correctly; no token/filesystem safety concerns introduced
  • no files require special attention

Important Files Changed

Filename Overview
lib/codex-manager.ts deleted inline runVerifyFlagged wrapper; added typed buildVerifyFlaggedCommandDeps() factory; both cli-dispatch and login-menu now call runVerifyFlaggedCommand directly — no behavioral change
test/codex-manager-cli.test.ts adds integration test for verify-flagged via login menu; covers the still-flagged path (failed refresh → save flagged list, no storage write); transaction mock and save mock assertions are consistent with command logic

Sequence Diagram

sequenceDiagram
    participant CLI as runCodexMultiAuthCli
    participant BLD as buildVerifyFlaggedCommandDeps()
    participant CMD as runVerifyFlaggedCommand()
    participant STORAGE as storage (mocked)
    participant REFRESH as queuedRefresh (mocked)

    Note over CLI: codex auth verify-flagged
    CLI->>BLD: construct deps object
    BLD-->>CLI: VerifyFlaggedCommandDeps
    CLI->>CMD: runVerifyFlaggedCommand(args, deps)
    CMD->>STORAGE: loadFlaggedAccounts()
    CMD->>REFRESH: queuedRefresh(token)
    REFRESH-->>CMD: TokenResult
    CMD->>STORAGE: withAccountAndFlaggedStorageTransaction(...) [if restore]
    CMD->>STORAGE: saveFlaggedAccounts(...) [if flaggedChanged && !storageChanged]
    CMD-->>CLI: exit code

    Note over CLI: login menu → verify-flagged action
    CLI->>BLD: construct deps object
    BLD-->>CLI: VerifyFlaggedCommandDeps
    CLI->>CMD: runVerifyFlaggedCommand([], deps)
    CMD->>STORAGE: loadFlaggedAccounts()
    CMD->>REFRESH: queuedRefresh(token)
    REFRESH-->>CMD: TokenResult
    CMD->>STORAGE: withAccountAndFlaggedStorageTransaction(...)
    CMD->>STORAGE: saveFlaggedAccounts(...)
    CMD-->>CLI: exit code (ignored — side-effect only)
Loading

Reviews (2): Last reviewed commit: "refactor: share verify-flagged command d..." | Re-trigger Greptile

@chatgpt-codex-connector
Copy link

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 2 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 78e50e5e-7a10-4c2e-934d-c5b3248ccb23

📥 Commits

Reviewing files that changed from the base of the PR and between 3b415fe and 7c1507d.

📒 Files selected for processing (2)
  • lib/codex-manager.ts
  • test/codex-manager-cli.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/pr1-route-verify-flagged-direct
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch refactor/pr1-route-verify-flagged-direct

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ndycode
Copy link
Owner Author

ndycode commented Mar 23, 2026

Closing because this work is now included in main via #318 and #319.

@ndycode ndycode closed this Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant